Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

show more context for errors from fluxctl #1615

Merged
merged 2 commits into from
Dec 27, 2018

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Dec 25, 2018

Errors in fluxctl are unwrapped before printing since we want to
format flux-specific errors specially, and they can come wrapped.

However, the unwrapping can lead to losing context when printing
non-flux-specific errors.

For instance, after
https://github.com/justinbarrick/go-k8s-portforward/pulls/3 we display
the following when a kubeconfig file isn't found:

Error: invalid configuration: no configuration has been provided
Run 'fluxctl identity --help' for usage.

Which is the cause of a much more helpful error when in full:

Error: Could not load kubernetes configuration file: invalid configuration: no configuration has been provided
Run 'fluxctl identity --help' for usage.

This change removes the unwrapping for non-flux-specific errors.

Errors in fluxctl are unwrapped before printing since we want to
format flux-specific errors specially, and they can come wrapped.

However, the unwrapping can lead to losing context when printing
non-flux-specific errors.

For instance, after
https://github.com/justinbarrick/go-k8s-portforward/pulls/3 we display
the following when a kubeconfig file isn't found:

Error: invalid configuration: no configuration has been provided
Run 'fluxctl identity --help' for usage.

Which is the cause of a much more helpful error when in full:

Error: Could not load kubernetes configuration file: invalid configuration: no configuration has been provided
Run 'fluxctl identity --help' for usage.

This change removes the unwrapping for non-flux-specific errors.
@squaremo
Copy link
Member

This changes the behaviour when there's a *fluxerr.Error -- it'll print the help text, then the error message, then the usage, while before it just printed the help text. I don't think this is wrong, just flagging it up. This is what it looks like:

$ FLUX_URL=https://cloud.weave.works/api/flux fluxctl identity
== Error ==

The request failed authentication

This most likely means you have a missing or incorrect token. Please
make sure you supply a service token, either by setting the
environment variable FLUX_SERVICE_TOKEN, or using the argument --token
with fluxctl.


Error: executing HTTP request: request failed authentication
Run 'fluxctl identity --help' for usage.

Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes things better 👍

@2opremio
Copy link
Contributor Author

This is what it looks like:

$ FLUX_URL=https://cloud.weave.works/api/flux fluxctl identity
== Error ==

The request failed authentication

This most likely means you have a missing or incorrect token. Please
make sure you supply a service token, either by setting the
environment variable FLUX_SERVICE_TOKEN, or using the argument --token
with fluxctl.



Error: executing HTTP request: request failed authentication
Run 'fluxctl identity --help' for usage.

My intention was to unify the error output, but in hindsight it just provides redundant information, so I reverted to the original behavior.

@2opremio 2opremio merged commit f0eac80 into fluxcd:master Dec 27, 2018
@2opremio 2opremio deleted the fluxctl-error-context branch December 27, 2018 22:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants